[clang][x86][bytecode] Replace interp__builtin_parity/clrsb/bitreverse/ffs with static bool interp__builtin_elementwise_int_unaryop callback #162346

donneypr · 2025-10-07T19:17:19Z

Fixes #160288

llvmbot · 2025-10-07T19:17:56Z

@llvm/pr-subscribers-clang

Author: don (donneypr)

Changes

Fixes #160288

Full diff: https://github.com/llvm/llvm-project/pull/162346.diff

1 Files Affected:

(modified) clang/lib/AST/ByteCode/InterpBuiltin.cpp (+12-32)

diff --git a/clang/lib/AST/ByteCode/InterpBuiltin.cpp b/clang/lib/AST/ByteCode/InterpBuiltin.cpp
index a0dcdace854b9..81e6bd55990fa 100644
--- a/clang/lib/AST/ByteCode/InterpBuiltin.cpp
+++ b/clang/lib/AST/ByteCode/InterpBuiltin.cpp
@@ -671,33 +671,6 @@ static bool interp__builtin_popcount(InterpState &S, CodePtr OpPC,
   return true;
 }
 
-static bool interp__builtin_parity(InterpState &S, CodePtr OpPC,
-                                   const InterpFrame *Frame,
-                                   const CallExpr *Call) {
-  PrimType ArgT = *S.getContext().classify(Call->getArg(0)->getType());
-  APSInt Val = popToAPSInt(S.Stk, ArgT);
-  pushInteger(S, Val.popcount() % 2, Call->getType());
-  return true;
-}
-
-static bool interp__builtin_clrsb(InterpState &S, CodePtr OpPC,
-                                  const InterpFrame *Frame,
-                                  const CallExpr *Call) {
-  PrimType ArgT = *S.getContext().classify(Call->getArg(0)->getType());
-  APSInt Val = popToAPSInt(S.Stk, ArgT);
-  pushInteger(S, Val.getBitWidth() - Val.getSignificantBits(), Call->getType());
-  return true;
-}
-
-static bool interp__builtin_bitreverse(InterpState &S, CodePtr OpPC,
-                                       const InterpFrame *Frame,
-                                       const CallExpr *Call) {
-  PrimType ArgT = *S.getContext().classify(Call->getArg(0)->getType());
-  APSInt Val = popToAPSInt(S.Stk, ArgT);
-  pushInteger(S, Val.reverseBits(), Call->getType());
-  return true;
-}
-
 static bool interp__builtin_classify_type(InterpState &S, CodePtr OpPC,
                                           const InterpFrame *Frame,
                                           const CallExpr *Call) {
@@ -3032,18 +3005,25 @@ bool InterpretBuiltin(InterpState &S, CodePtr OpPC, const CallExpr *Call,
   case Builtin::BI__builtin_parity:
   case Builtin::BI__builtin_parityl:
   case Builtin::BI__builtin_parityll:
-    return interp__builtin_parity(S, OpPC, Frame, Call);
-
+    return interp__builtin_elementwise_int_unaryop(
+        S, OpPC, Call, [](const APSInt &Val) -> APInt {
+          return APInt(Val.getBitWidth(), Val.popcount() % 2);
+        });
   case Builtin::BI__builtin_clrsb:
   case Builtin::BI__builtin_clrsbl:
   case Builtin::BI__builtin_clrsbll:
-    return interp__builtin_clrsb(S, OpPC, Frame, Call);
-
+    return interp__builtin_elementwise_int_unaryop(
+        S, OpPC, Call, [](const APSInt &Val) -> APInt {
+          return APInt(Val.getBitWidth(),
+                       Val.getBitWidth() - Val.getSignificantBits());
+        });
   case Builtin::BI__builtin_bitreverse8:
   case Builtin::BI__builtin_bitreverse16:
   case Builtin::BI__builtin_bitreverse32:
   case Builtin::BI__builtin_bitreverse64:
-    return interp__builtin_bitreverse(S, OpPC, Frame, Call);
+    return interp__builtin_elementwise_int_unaryop(
+        S, OpPC, Call,
+        [](const APSInt &Val) -> APInt { return Val.reverseBits(); });
 
   case Builtin::BI__builtin_classify_type:
     return interp__builtin_classify_type(S, OpPC, Frame, Call);

github-actions · 2025-10-07T19:44:43Z

✅ With the latest revision this PR passed the C/C++ code formatter.

donneypr · 2025-10-07T20:18:22Z

/home/gha/actions-runner/_work/llvm-project/llvm-project/clang/lib/AST/ByteCode/InterpBuiltin.cpp:716:13: error: unused function 'interp__builtin_rotate' [-Werror,-Wunused-function]

Looks like this function is not used thus the Linux Build and Test failing, please let me know if this is accurate.

donneypr · 2025-10-07T20:37:43Z

/home/gha/actions-runner/_work/llvm-project/llvm-project/clang/lib/AST/ByteCode/InterpBuiltin.cpp:716:13: error: unused function 'interp__builtin_rotate' [-Werror,-Wunused-function]

Looks like this function is not used thus the Linux Build and Test failing, please let me know if this is accurate.

Nevermind, I think that was accidentally added when I was merging. The callback for interp__builtin_rotate was replaced in #160289.

donneypr · 2025-10-07T20:57:35Z

@tbaederr @RKSimon Can I please get a review? Thank you 😊 !!

clang/lib/AST/ByteCode/InterpBuiltin.cpp

RKSimon

LGTM - cheers

donneypr · 2025-10-08T14:17:30Z

Thank you @RKSimon @tbaederr , have a great rest of your day!!

…e/ffs with static bool interp__builtin_elementwise_int_unaryop callback (#162346) Fixes #160288

…e/ffs with static bool interp__builtin_elementwise_int_unaryop callback (llvm#162346) Fixes llvm#160288

shafik · 2025-10-10T22:39:09Z

clang/lib/AST/ByteCode/InterpBuiltin.cpp

  case Builtin::BI__builtin_ffsll:
-    return interp__builtin_ffs(S, OpPC, Frame, Call);
+    return interp__builtin_elementwise_int_unaryop(
+        S, OpPC, Call, [](const APSInt &Val) {


Curious, you only left off the trailing return type on this one.

Missed it on my commit. This should be fixed, thank you for pointing it out. I can get it fixed with my next PR.

@shafik It doesn't look like any of the interp__builtin_elementwise_int_unaryop calls require a trailing type - why do you want it adding here instead of stripping it off the others?

My assumption was that in the Val.reverseBits() case it was not obvious and it was added to the others for uniformity and the fourth was accidently left out. The PR was accepted so this felt like a valid take. I would also be consistent if you removed it from all of them.

@tbaederr how do you feel?

I usually add a trailing return type when I write lambdas, but I think that's the exception when looking at the entire code base, so I don't mind either way.

Just my novice 2 cents: If we're keeping the trailing return type for some of the functions eg. Val.reverseBits(), I think it would better to add it to all of them for uniformity. I wasn't going to add it initially but a previous PR had the trailing type so I used it to stay consistent.

…e/ffs with static bool interp__builtin_elementwise_int_unaryop callback (llvm#162346) Fixes llvm#160288

…tin_elementwise_int_unaryop callbacks (#163905) Regarding the discussion in #162346, this PR is to remove the trailing type from the 'interp__builtin_elementwise_int_unaryop' callbacks.

donneypr added 4 commits October 7, 2025 14:45

Replaced callback for parity llvm#160288

26eada5

Replaced callback for clrsb llvm#160288

9d8d082

Replaced callback for bitreverse llvm#160288

d5e1e63

fixed typo for clrsb V -> Val

b08aa33

llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" clang:bytecode Issues for the clang bytecode constexpr interpreter labels Oct 7, 2025

donneypr marked this pull request as draft October 7, 2025 19:20

Replaced callback for ffs llvm#160288

8d9c86c

donneypr marked this pull request as ready for review October 7, 2025 19:41

Merge branch 'main' into clang/bytecode-unaryop-refactor

f1b6cdf

clang formatted

ac30e63

donneypr added 2 commits October 7, 2025 16:28

Bytecode: fix rotate; drop accidental ffs lines

08f30d2

removed rotate as it was removed in PR llvm#161924

8089053

RKSimon requested review from RKSimon and tbaederr October 7, 2025 21:18

tbaederr reviewed Oct 8, 2025

View reviewed changes

clang/lib/AST/ByteCode/InterpBuiltin.cpp Show resolved Hide resolved

Check to see if Val is 0 for ffs llvm#160288

b5d1c9f

RKSimon approved these changes Oct 8, 2025

View reviewed changes

RKSimon enabled auto-merge (squash) October 8, 2025 14:12

donneypr requested a review from tbaederr October 8, 2025 14:13

tbaederr approved these changes Oct 8, 2025

View reviewed changes

RKSimon merged commit d0da857 into llvm:main Oct 8, 2025
9 checks passed

svkeerthy pushed a commit that referenced this pull request Oct 9, 2025

[clang][x86][bytecode] Replace interp__builtin_parity/clrsb/bitrevers…

14fc8c7

…e/ffs with static bool interp__builtin_elementwise_int_unaryop callback (#162346) Fixes #160288

clingfei pushed a commit to clingfei/llvm-project that referenced this pull request Oct 10, 2025

[clang][x86][bytecode] Replace interp__builtin_parity/clrsb/bitrevers…

766e1a2

…e/ffs with static bool interp__builtin_elementwise_int_unaryop callback (llvm#162346) Fixes llvm#160288

shafik reviewed Oct 10, 2025

View reviewed changes

akadutta pushed a commit to akadutta/llvm-project that referenced this pull request Oct 14, 2025

[clang][x86][bytecode] Replace interp__builtin_parity/clrsb/bitrevers…

ef91fdc

…e/ffs with static bool interp__builtin_elementwise_int_unaryop callback (llvm#162346) Fixes llvm#160288

donneypr added a commit to donneypr/llvm-project that referenced this pull request Oct 17, 2025

added trailing type for ffs, missed in pr llvm#162346 for llvm#160288

c84d839

donneypr mentioned this pull request Oct 17, 2025

[clang][x86][bytecode] remove trailing returns type from interp__builtin_elementwise_int_unaryop callbacks #163905

Merged

[clang][x86][bytecode] Replace interp__builtin_parity/clrsb/bitreverse/ffs with static bool interp__builtin_elementwise_int_unaryop callback #162346

[clang][x86][bytecode] Replace interp__builtin_parity/clrsb/bitreverse/ffs with static bool interp__builtin_elementwise_int_unaryop callback #162346

Uh oh!

Conversation

donneypr commented Oct 7, 2025

Uh oh!

llvmbot commented Oct 7, 2025

Uh oh!

github-actions bot commented Oct 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

donneypr commented Oct 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

donneypr commented Oct 7, 2025

Uh oh!

donneypr commented Oct 7, 2025

Uh oh!

Uh oh!

RKSimon left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

donneypr commented Oct 8, 2025

Uh oh!

shafik Oct 10, 2025

Choose a reason for hiding this comment

Uh oh!

donneypr Oct 14, 2025

Choose a reason for hiding this comment

Uh oh!

RKSimon Oct 14, 2025

Choose a reason for hiding this comment

Uh oh!

shafik Oct 14, 2025

Choose a reason for hiding this comment

Uh oh!

tbaederr Oct 15, 2025

Choose a reason for hiding this comment

Uh oh!

donneypr Oct 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

github-actions bot commented Oct 7, 2025 •

edited

Loading

donneypr commented Oct 7, 2025 •

edited

Loading

donneypr Oct 15, 2025 •

edited

Loading